Add support for rendering more logical classsynopsis markup #77

Girgias · 2023-07-06T15:34:06Z

while allowing the old markup to render.

I'll push an example of some doc-en changes that would render this markup.

From my testing (which is not the most extensive) nothing is broken.

…owing the old markup to render

mallardduck · 2023-07-06T17:03:17Z

So far the only "issue" i'm seeing is that this doesn't remove the redundant name from methods. AFAIK the current behaviour there is the desired result, one where local methods do not have the prefix of the class/interface they are within.

I think correcting this would require duplicating some of what is done for classes currently. So when the method is processed by format_classsynopsis_methodsynopsis_methodname_text it needs a reference of the current class like $this->cchunk["classsynopsis"]["classname"]. Doing a quick dump debug it looks like this is the default value of false instead of the current Interface name. Might be worth introducing a new interfacename field unless you just want to reuse the classname.

Girgias · 2023-07-07T02:18:27Z

So far the only "issue" i'm seeing is that this doesn't remove the redundant name from methods. AFAIK the current behaviour there is the desired result, one where local methods do not have the prefix of the class/interface they are within.

I think correcting this would require duplicating some of what is done for classes currently. So when the method is processed by format_classsynopsis_methodsynopsis_methodname_text it needs a reference of the current class like $this->cchunk["classsynopsis"]["classname"]. Doing a quick dump debug it looks like this is the default value of false instead of the current Interface name. Might be worth introducing a new interfacename field unless you just want to reuse the classname.

Yeah that was the issue, I reused the classname field as otherwise when you have an interface list it would potentially override stuff.

Also added support for <ooexception> while I was there and did some refactoring to group stuff together.

Girgias · 2023-07-07T02:21:17Z

phpdotnet/phd/Package/PHP/XHTML.php

-        'classname'             => array(
-            /* DEFAULT */          'span',
-            'ooclass'           => array(
-                /* DEFAULT */      'format_suppressed_tags',
-                'classsynopsisinfo' => 'format_classsynopsisinfo_ooclass_classname',
-            ),
-        ),


For reference, this is useless, as it duplicated the same information from the generic XHTML format.

mallardduck · 2023-07-07T16:10:03Z

Think this is looking pretty good from testing on my end. Only "issue" is that (php/doc-en#2563) is missing my previous changes for DS interfaces. So it reverts some items back to abstract modifiers I had removed from affected DS interface methods. Maybe not a big deal tho since rebase should fix that and I'll need to make adjustments so those work with this PR.

Girgias · 2023-07-10T08:20:54Z

Think this is looking pretty good from testing on my end. Only "issue" is that (php/doc-en#2563) is missing my previous changes for DS interfaces. So it reverts some items back to abstract modifiers I had removed from affected DS interface methods. Maybe not a big deal tho since rebase should fix that and I'll need to make adjustments so those work with this PR.

Issue is because I guard the legacy renderer from the use of the class attribute :/ But I think before merging this I'll try to add support to gen_stub and run a whole batch of these again. (Although IIRC DS does not use stub files yet within the extension)

Girgias · 2023-07-26T13:10:40Z

@mallardduck merged this so feel free to fix the DS markup again. :)

mallardduck · 2023-07-26T14:04:34Z

@Girgias - does gen_stub need to still be updated and ran on things? If I follow correctly that should cover core PHP things but not extensions right?

Currently I am hitting the assertion you put in here:
https://github.com/php/phd/blob/master/phpdotnet/phd/Package/Generic/XHTML.php#L1009

It seems to be being caused by Traversable having the class attribute, but using the structure from my PRs method. So it needs to be updated to match the syntax you've sorted out for us. Howveer should I update those by hand too, or is that what gen_stub should cover?

Girgias · 2023-07-26T14:12:28Z

@kocsismate is covering gen_stub, so don't worry about that.

No, you don't need to convert any classes that use the "old" rendering, the example changes I made for Traversable are just an example of the markup and I don't really indent on changing them as that is going to be a lot of churn for translation for the time being.

I would "just" change the interfaces for the time being to the new markup.

Girgias · 2023-07-26T14:13:15Z

Also gen_stub can cover PECL extensions (as I did that for ibm_db2) but it needs stub files, which last time I checked DS did not have.

mallardduck · 2023-07-26T14:18:19Z

Ok - so is it expected that I should see build errors currently then? I'm seeing:

php phd/render.php --docbook doc-base/.manual.xml --package PHP --format php
[14:15:25 - Rendering Style       ] Running full build
[14:15:25 - Indexing              ] Skipping indexing
[14:15:25 - Rendering Style       ] Running full build
[14:15:25 - Rendering Format      ] Starting PHP-Web rendering

Fatal error: Uncaught AssertionError: assert($this->cchunk['classsynopsis']['legacy'] === true) in /Users/danpock/GitProjects/the-php/phd/phpdotnet/phd/Package/Generic/XHTML.php:1009
Stack trace:
#0 /Users/danpock/GitProjects/the-php/phd/phpdotnet/phd/Package/Generic/XHTML.php(1009): assert(false, 'assert($this->c...')
#1 /Users/danpock/GitProjects/the-php/phd/phpdotnet/phd/Render.php(123): phpdotnet\phd\Package_Generic_XHTML->format_classsynopsisinfo(false, 'classsynopsisin...', Array, Array)
#2 /Users/danpock/GitProjects/the-php/phd/render.php(121): phpdotnet\phd\Render->execute(Object(phpdotnet\phd\Reader))
#3 {main}
  thrown in /Users/danpock/GitProjects/the-php/phd/phpdotnet/phd/Package/Generic/XHTML.php on line 1009

When I add a try-catch with good old dump-die, I see it's from Traversable:

array(23) {
  ["classsynopsis"]=>
  array(7) {
    ["close"]=>
    bool(false)
    ["classname"]=>
    string(11) "Traversable"

Girgias · 2023-07-26T14:27:24Z

AHHH right I had forgotten those got updated to use your rendering >_> yeah those need to be fixed... I suppose let's just wait for @kocsismate to finish updating gen_stub so that we can fix those interfaces too

Following php/phd#77

Lewiscowles1986 · 2023-08-03T08:30:48Z

I Just rebased php/doc-en#2638 atop @kocsismate branch and it looks like the error still happens

Fatal error: Uncaught AssertionError: assert($this->cchunk['classsynopsis']['legacy'] === true) in /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php:1009
Stack trace:
#0 /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php(1009): assert(false, 'assert($this->c...')
#1 /var/www/phd/phpdotnet/phd/Render.php(125): phpdotnet\phd\Package_Generic_XHTML->format_classsynopsisinfo(false, 'classsynopsisin...', Array, Array)
#2 /var/www/phd/render.php(121): phpdotnet\phd\Render->execute(Object(phpdotnet\phd\Reader))
#3 {main}
  thrown in /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php on line 1009

Is there more work to do in #2611 or is there a way to make a branch from before this PR merged?

Girgias · 2023-08-03T11:29:31Z

I Just rebased php/doc-en#2638 atop @kocsismate branch and it looks like the error still happens

Fatal error: Uncaught AssertionError: assert($this->cchunk['classsynopsis']['legacy'] === true) in /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php:1009
Stack trace:
#0 /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php(1009): assert(false, 'assert($this->c...')
#1 /var/www/phd/phpdotnet/phd/Render.php(125): phpdotnet\phd\Package_Generic_XHTML->format_classsynopsisinfo(false, 'classsynopsisin...', Array, Array)
#2 /var/www/phd/render.php(121): phpdotnet\phd\Render->execute(Object(phpdotnet\phd\Reader))
#3 {main}
  thrown in /var/www/phd/phpdotnet/phd/Package/Generic/XHTML.php on line 1009

Is there more work to do in #2611 or is there a way to make a branch from before this PR merged?

Are you sure you have the latest commits from doc-base, doc-en, and phd?

As I can render the docs without any issues.

Commands used from my phd clone:

php ../base/configure.php 
php render.php -d ../base/.manual.xml -P PHP -f xhtml -o ../output/

Lewiscowles1986 · 2023-08-03T11:53:12Z

So I'm running from outside of the phd repo, but I am on master for phd, doc-base and doc-en.

This is also affecting another builder, who instead of highlighting was working around using a commit pin.

I can confirm that if I CD into phd, and copy-paste your commands I still get this error.

Lewiscowles1986 · 2023-08-03T11:56:37Z

doc-en commit hash 184082814f268f881bf7834c5400e1981140d8ea (master HEAD)
doc-base commit hash 15be9f9b25fe4b4b553bd7aed973966ba8103f4e (master HEAD)
phpd commit hash 619c1c1a68dbc04c42e6b9216a61701a3b076a22 (master HEAD)

Girgias · 2023-08-03T12:10:43Z

Can you change the:
assert($this->cchunk["classsynopsis"]["legacy"] === true); on line 1009 of phd/Package/Generic/XHTML.php to

if (!$this->cchunk["classsynopsis"]["legacy"] === true) { var_dump($name); }

This might give some info without needed to go through a debugger

mallardduck · 2023-08-03T12:14:50Z

@Girgias is this still waiting on @kocsismate to update gen_stub and rerun that? It seems a lot like what I reported last week.

Girgias · 2023-08-03T12:17:53Z

@Girgias is this still waiting on @kocsismate to update gen_stub and rerun that? It seems a lot like what I reported last week.

It has been updated, and the interfaces have been updated. Just classes still use the "legacy" rendering for the most part, which is somewhat incompatible with the output as I want to let translation some time to catch up.

Lewiscowles1986 · 2023-08-03T13:11:10Z

@kocsismate has not yet been merged, but I have done a 3 way branch including master, kocsismate and the new code, as well as tested this on just kocsismate and master. It would really help me to know which sha's are being used for the instructions above and if it works in a clean-install, or if there are maybe files or instructions I'm not following.

I Started this to troubleshoot if there was a nicer way to handle https://github.com/php/doc-en/pull/2638/files#diff-38801fa32c4bd7bd97aa3c87c138be36e126e0337b22aacf27ab57365357dc20R10 as I think being able to build from master, branch or tag (all of which I can clone shallow) would work.

The linked PR is not my work, but only works (same error as I've shown here) if pinning to a specific commit sha.

mallardduck · 2023-08-03T13:13:38Z

@Lewiscowles1986 - looks like the current build errors are coming from DS needing to be updated for the new format. I'll send the follow up PR to update those for @Girgias syntax improvements.

Lewiscowles1986 · 2023-08-03T13:15:10Z

Thanks all and sorry for creating such noise on a merged branch 👍 . I'd love to know what DS is?

mallardduck · 2023-08-03T13:58:54Z

@Girgias - sent a PR: php/doc-en#2641 to fix the DS docs. Seems to fix builds!

○ → php phd/render.php --docbook doc-base/.manual.xml --package PHP --format php
[13:56:40 - Rendering Style       ] Running full build
[13:56:40 - Indexing              ] Indexing...
[13:56:49 - Indexing              ] Indexing done
[13:56:49 - Rendering Style       ] Running full build
[13:56:49 - Rendering Format      ] Starting PHP-Web rendering
[13:57:07 - Rendering Format      ] Writing search indexes..
[13:57:07 - Rendering Format      ] Index written
[13:57:07 - Rendering Format      ] Finished rendering

@Lewiscowles1986 - just shorthand for Data Structures extension. https://www.php.net/manual/en/intro.ds.php

Girgias · 2023-08-03T14:01:01Z

ACK currently not in front of a computer but will do it ASAP

Follow-up on php/phd#77

Add support for rendering more logical classsynopsis markup while all…

092fdac

…owing the old markup to render

This was referenced Jul 6, 2023

Example of new markup for PhD php/doc-en#2563

Closed

Rendering issues for Interfaces that extend another one #76

Closed

Fix interface names not being removed from methods + exceptions

ad7e0e4

Girgias commented Jul 7, 2023

View reviewed changes

Girgias merged commit ac72a94 into master Jul 26, 2023

Girgias deleted the approach-2-fix-classsynopsis branch July 26, 2023 13:10

This was referenced Jul 27, 2023

Generate the new class synopsis markup php/php-src#11809

Merged

Use new class synopsis markup php/doc-en#2611

Merged

kocsismate added a commit to kocsismate/doc-en that referenced this pull request Jul 27, 2023

Use new class synopsis markup

e3127dc

Following php/phd#77

kocsismate added a commit to kocsismate/doc-en that referenced this pull request Jul 31, 2023

Use new class synopsis markup

b851d14

Following php/phd#77

Lewiscowles1986 mentioned this pull request Aug 3, 2023

Create a docker container for a single-line setup and rendering php/doc-en#2638

Merged

Girgias pushed a commit to php/doc-en that referenced this pull request Aug 30, 2023

Use new class synopsis markup (#2611)

14767af

Follow-up on php/phd#77

alamirault mentioned this pull request Sep 21, 2023

Fatal error: Uncaught AssertionError on docker compose up php/doc-en#2763

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for rendering more logical classsynopsis markup #77

Add support for rendering more logical classsynopsis markup #77

Girgias commented Jul 6, 2023

mallardduck commented Jul 6, 2023 •

edited

Loading

Girgias commented Jul 7, 2023

Girgias Jul 7, 2023

mallardduck commented Jul 7, 2023

Girgias commented Jul 10, 2023

Girgias commented Jul 26, 2023

mallardduck commented Jul 26, 2023 •

edited

Loading

Girgias commented Jul 26, 2023

Girgias commented Jul 26, 2023

mallardduck commented Jul 26, 2023

Girgias commented Jul 26, 2023

Lewiscowles1986 commented Aug 3, 2023

Girgias commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

Girgias commented Aug 3, 2023

mallardduck commented Aug 3, 2023 •

edited

Loading

Girgias commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023

mallardduck commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

mallardduck commented Aug 3, 2023 •

edited

Loading

Girgias commented Aug 3, 2023

Add support for rendering more logical classsynopsis markup #77

Add support for rendering more logical classsynopsis markup #77

Conversation

Girgias commented Jul 6, 2023

mallardduck commented Jul 6, 2023 • edited Loading

Girgias commented Jul 7, 2023

Girgias Jul 7, 2023

Choose a reason for hiding this comment

mallardduck commented Jul 7, 2023

Girgias commented Jul 10, 2023

Girgias commented Jul 26, 2023

mallardduck commented Jul 26, 2023 • edited Loading

Girgias commented Jul 26, 2023

Girgias commented Jul 26, 2023

mallardduck commented Jul 26, 2023

Girgias commented Jul 26, 2023

Lewiscowles1986 commented Aug 3, 2023

Girgias commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023 • edited Loading

Lewiscowles1986 commented Aug 3, 2023 • edited Loading

Girgias commented Aug 3, 2023

mallardduck commented Aug 3, 2023 • edited Loading

Girgias commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023

mallardduck commented Aug 3, 2023

Lewiscowles1986 commented Aug 3, 2023 • edited Loading

mallardduck commented Aug 3, 2023 • edited Loading

Girgias commented Aug 3, 2023

mallardduck commented Jul 6, 2023 •

edited

Loading

mallardduck commented Jul 26, 2023 •

edited

Loading

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

mallardduck commented Aug 3, 2023 •

edited

Loading

Lewiscowles1986 commented Aug 3, 2023 •

edited

Loading

mallardduck commented Aug 3, 2023 •

edited

Loading